feat: add scrollbar to table of contents on long articles#275
feat: add scrollbar to table of contents on long articles#275AnujShrivastava01 wants to merge 4 commits intokeploy:mainfrom
Conversation
Signed-off-by: Anuj Shrivastava <anujshrivastava10e@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a scrollable table of contents (TOC) for long articles on desktop layouts, preventing the TOC from extending beyond the viewport. The implementation restricts the TOC height to 80% of viewport height and enables vertical scrolling when content exceeds this limit.
Changes:
- Added scrolling behavior to the desktop TOC navigation element with Tailwind utility classes
- Applied max-height constraint, overflow scrolling, and padding for scrollbar spacing
Comments suppressed due to low confidence (1)
components/TableContents.tsx:59
- The resizeHandler logic at line 58 checks if
container.clientHeight > window.innerHeight * 0.8to determine whether to show a dropdown list. However, with the newmax-h-[80vh]constraint applied to the nav element, the container's clientHeight will never exceed 80vh (0.8 * window.innerHeight), making this condition always false. This breaks the intended behavior of switching to a dropdown when the TOC is too tall. The logic should check the scrollHeight instead of clientHeight, or the threshold should be adjusted to account for the max-height constraint.
function resizeHandler() {
setIsList(container.clientHeight > window.innerHeight * 0.8);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Anuj Shrivastava <anujshrivastava10e@gmail.com>
Signed-off-by: Anuj Shrivastava <anujshrivastava10e@gmail.com>
amaan-bhati
left a comment
amaan-bhati
left a comment
There was a problem hiding this comment.
Hey @AnujShrivastava01 Thanks for raising this pr, before i review this on local, can you please also share a preview screenshot for how it looks in smaller screen sizes as well?
|
Hii @amaan-bhati Sir, thanks for checking this out! I’ve added preview screenshots for smaller screen sizes in the PR description above. The layout has been tested to ensure proper spacing, alignment, and responsiveness on mobile and small-width viewports.
|
|
@amaan-bhati Sir, please review the changes. Thank you. |
|
@Achanandhi-M Sir, please review the PR. Thank you. |
|
Hey @AnujShrivastava01 👋 — thanks a lot for taking the time to contribute, we genuinely appreciate it! After reviewing this PR, we won't be able to merge it at this time. Here's the reason: Please don't be discouraged — we'd love to see more contributions from you. Feel free to open a new PR if you think this can be reworked, or reach out if you have any questions. Thanks again for your effort!😊 |
Pull Request: Add Scrollbar to Table of Contents
Description
Implements a scrollable Table of Contents component for long articles, improving user experience by allowing independent scrolling of TOC without affecting page scroll position.
Fixes keploy/keploy#3543
🔧 Changes Made
max-h-[80vh]to limit TOC height to 80% of viewport heightoverflow-y-autoto enable scrollbar when content exceeds max heightpr-2for proper scrollbar spacing and paddingFiles Changed
components/TableContents.tsxTesting
Screenshots/Preview
Before: TOC extends beyond viewport without scrollbar
After: TOC has scrollbar when content exceeds 80vh
Checklist